-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add array_element #23
Conversation
@@ -112,7 +112,22 @@ hocon_setting { 'sample map setting': | |||
type => 'array', | |||
} | |||
``` | |||
|
|||
|
|||
If you are trying to manage single entries in an array (for example, adding to an array from a define) you will need tot set the `'type'` parameter to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/tot/to
1f5ea7c
to
ead4c7e
Compare
@fpringvaldsen fixed |
tmp_val = [] | ||
tmp_val << value | ||
tmp_val << value_to_set | ||
tmp_val.flatten! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should be calling flatten!
and uniq!
on the array here? HOCON supports duplicate array elements as well as nested arrays, so I could see this possibly changing a user's configuration in unexpected ways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern with not having uniq!
for the array_element
is idempotence. Since the array can't be rewritten every time we'd just end up with duplicate elements each run. I think if I iterate through value
and value_to_set
I can get rid of flatten!
though.
@mhaskel Thanks! I'm 👍 on this aside from calling @jpinsonault or @cprice404 could one of you take a look at this PR? |
This allows elements in arrays in hocon files to managed individually.
993e8b9
to
fcd8970
Compare
@fpringvaldsen I was able to get around |
\o/ :+1: |
This seems good if we think this will be a common use case, but if it's being implemented specifically in support of the authorization rules stuff, I'm not quite understanding how it will deal with sort order? The sort order of the array will be important. |
@cprice404 I think this would be generally useful. WRT sorting I haven't thought of a more generic way to implement that yet, it seems like each use is going to have some specific needs. |
Cool, I'm +1 on it as a "generally useful" thing, just wanted to make sure the sorting requirement is still being addressed for the other module. |
This allows elements in arrays in hocon files to managed individually.